new_audit(errors-in-console): Added new runtime exceptions #3494
Conversation
| afterPass(options) { | ||
| return Promise.resolve() | ||
| .then(_ => options.driver.off('Runtime.exceptionThrown', this._onRuntimeExceptionThrown)) | ||
| .then(_ => options.driver.sendCommand('Runtime.disable')) |
There was a problem hiding this comment.
nuke this. we'll keep Runtime domain on
| RuntimeExceptions: [{ | ||
| 'timestamp': 1506535813608.003, | ||
| 'exceptionDetails': { | ||
| 'url': 'http://www.inmotionhosting.com/support/templates/supportcentertemplate/js/fancybox.js', |
There was a problem hiding this comment.
This test has a js failure dependency from this page, if they fix it it will start to fail, remove it as this is covered already from the new smoke test
There was a problem hiding this comment.
can these be switched to a more generic url instead of a real site? Something like 'http://example.com/fancybox.js' for the three instances of these should be fine
|
@siteriaitaliana can you rebase from master? I believe that will fix the appveyor error but there are some new (minor) linting errors that will need to be fixed |
| @@ -35,6 +35,11 @@ | |||
| <!-- FAIL: block rendering --> | |||
| <script src="./dbw_tester.js"></script> | |||
|
|
|||
| <!-- Runtime Ecception thrown here to test the new runtime exception gatherer. --> | |||
There was a problem hiding this comment.
switch to something like
<!-- FAIL(errors-in-console): exception thrown -->
| RuntimeExceptions: [{ | ||
| 'timestamp': 1506535813608.003, | ||
| 'exceptionDetails': { | ||
| 'url': 'http://www.inmotionhosting.com/support/templates/supportcentertemplate/js/fancybox.js', |
There was a problem hiding this comment.
can these be switched to a more generic url instead of a real site? Something like 'http://example.com/fancybox.js' for the three instances of these should be fine
| { | ||
| 'timestamp': 1506535813608.003, | ||
| 'exceptionDetails': { | ||
| 'url': 'http://www.inmotionhosting.com/support/templates/supportcentertemplate/js/fancybox.js', |
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
siteriaitaliana
force-pushed
the
runtime-exceptions-in-chrome-msg-gatherer
branch
from
fc46937
to
27e2d33
Compare
…clude js console errors code review actions
siteriaitaliana
force-pushed
the
runtime-exceptions-in-chrome-msg-gatherer
branch
from
27e2d33
to
d459e3c
Compare
siteriaitaliana
changed the title
siteriaitaliana
changed the title
siteriaitaliana
changed the title
|
@paulirish @brendankenny @patrickhulce guys I've actioned what requested looks good to me now |
|
awesome PR. thanks for this second round, @siteriaitaliana ! |
@paulirish @patrickhulce @brendankenny as promised this the 2nd part of the errors-in-console, this should include the js errors now